Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GH-448] Fix multiple SocketConnections in Overworld #489

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lotuuu
Copy link
Collaborator

@lotuuu lotuuu commented Jun 11, 2024

Closes #448

Motivation

A new SocketConnection was being instantiated every time we went into the Overworld scene, which started a new WS connection.

With this PR we singletonize the singleton (? to actually keep it as the unique instance.

We also need to fix the ServerSelect stuff a bit: if we called Init there, we would get an unexisting gameobject error. Instead, we use the currently existing singleton instance, connecting to the newly configured domain.

Summary of changes

  • Fix singleton pattern in SocketConnection.cs
  • Fix SelectServer.cs's usage of Socketconnection.cs

How has this been tested?

Play around with the swapping of servers and entering and leaving the Overworld scene. When connecting to localhost, you should see one [info] Websocket INIT called log in the server for each time you connected to the backend through the Server Selector, and not one for each time you entered the Overworld scene.

Checklist

  • I have tested the changes locally.
  • I self-reviewed the changes on GitHub, line by line.
  • Tests have been added/updated.
  • This change requires new documentation.
    • Documentation has been added/updated.
  • I have tested the changes in another devices.
    • Tested in iOS.
    • Tested in Android.

@lotuuu lotuuu self-assigned this Jun 11, 2024
@lotuuu lotuuu changed the title Fix multiple SocketConnections in Overworld [GH-448] Fix multiple SocketConnections in Overworld Jun 11, 2024
Copy link
Collaborator

@ncontinanza ncontinanza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

{
Debug.Log("SocketConnection ConnectToSession");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Debug.Log("SocketConnection ConnectToSession");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loading Overworld scene starts a new web socket connection
2 participants